Skip to content

Conversation

koelle25
Copy link
Contributor

@koelle25 koelle25 commented Aug 7, 2025

This PR fixes #6467.

Example API response value to test against:

{"rows":[{"uuid":"ad260e98-952f-47d8-a638-79a51803a8d1","enabled":"1","type":"primary","primaryip":"","forwardserver":"","transferkeyalgo":"","%transferkeyalgo":"None","transferkeyname":"","transferkey":"","allownotifysecondary":"","domainname":"example.com","allowtransfer":"","%allowtransfer":"","allowrndctransfer":"0","allowquery":"d66ade93-4d6b-4122-8c3f-8b4d16d31367","%allowquery":"everyone","allowrndcupdate":"1","serial":"2507310605","ttl":"300","refresh":"3600","retry":"900","expire":"86400","negative":"3600","mailadmin":"support.example.com","dnsserver":"ns1.example.com"},{"uuid":"eb1490ef-53b5-41ac-ac13-14132a4fda29","enabled":"1","type":"primary","primaryip":"","forwardserver":"","transferkeyalgo":"","%transferkeyalgo":"None","transferkeyname":"","transferkey":"","allownotifysecondary":"","domainname":"example.net","allowtransfer":"","%allowtransfer":"","allowrndctransfer":"0","allowquery":"d66ade93-4d6b-4122-8c3f-8b4d16d31367","%allowquery":"everyone","allowrndcupdate":"1","serial":"2507310605","ttl":"300","refresh":"3600","retry":"900","expire":"86400","negative":"3600","mailadmin":"support.example.net","dnsserver":"ns1.example.net"},{"uuid":"e8a80190-00d9-480f-8312-70d317584ce3","enabled":"1","type":"primary","primaryip":"","forwardserver":"","transferkeyalgo":"","%transferkeyalgo":"None","transferkeyname":"","transferkey":"","allownotifysecondary":"","domainname":"another.eu","allowtransfer":"","%allowtransfer":"","allowrndctransfer":"1","allowquery":"d66ade93-4d6b-4122-8c3f-8b4d16d31367","%allowquery":"everyone","allowrndcupdate":"1","serial":"2508070829","ttl":"300","refresh":"3600","retry":"900","expire":"86400","negative":"3600","mailadmin":"support.another.eu","dnsserver":"ns1.another.eu"}],"rowCount":3,"total":3,"current":1}

In the following lines we want to search for "example.net" (secondy entry in the example API response above).

There are now additional values between "type":"primary" and "domainname":"<domain>" in the API response.
Therefore I added a .* in the _grep_o regex string.

Due to this and the current API response structure the grep result now not only contains the searched domain, but all others before it, too (=> $string_with_searched_domain_and_others_before_it).
So, the "list entry" containing the searched domain is at the end of the string. Example:

"uuid":"ad260e98-952f-47d8-a638-79a51803a8d1","enabled":"1","type":"primary","primaryip":"","forwardserver":"","transferkeyalgo":"","%transferkeyalgo":"None","transferkeyname":"","transferkey":"","allownotifysecondary":"","domainname":"example.com","allowtransfer":"","%allowtransfer":"","allowrndctransfer":"0","allowquery":"d66ade93-4d6b-4122-8c3f-8b4d16d31367","%allowquery":"everyone","allowrndcupdate":"1","serial":"2507310605","ttl":"300","refresh":"3600","retry":"900","expire":"86400","negative":"3600","mailadmin":"support.example.com","dnsserver":"ns1.example.com"},{"uuid":"eb1490ef-53b5-41ac-ac13-14132a4fda29","enabled":"1","type":"primary","primaryip":"","forwardserver":"","transferkeyalgo":"","%transferkeyalgo":"None","transferkeyname":"","transferkey":"","allownotifysecondary":"","domainname":"example.net"

We'd need to cut the string on the last occurence of a { (which is before the uuid), but cut doesn't natively support that. To solve this, I reversed the string, then cut it on the first occurence of a {, then reversed it again to get it back into the correct order.
This selects only the last entry in the "list" from before, so only the domain we searched for (=> $string_with_searched_domain_only). Example:

"uuid":"eb1490ef-53b5-41ac-ac13-14132a4fda29","enabled":"1","type":"primary","primaryip":"","forwardserver":"","transferkeyalgo":"","%transferkeyalgo":"None","transferkeyname":"","transferkey":"","allownotifysecondary":"","domainname":"example.net"

Then the UUID gets extracted from this string as before, no change from my side (cut -d ':' -f 2 | cut -d '"' -f 2).

A test via the GitHub actions is not possible, as the OPNsense is a private instance only available in my internal network (as it should be).

Copy link

github-actions bot commented Aug 7, 2025

Welcome
READ ME !!!!!
Read me !!!!!!
First thing: don't send PR to the master branch, please send to the dev branch instead.
Please read the DNS API Dev Guide.
You MUST pass the DNS-API-Test.
Then reply on this message, otherwise, your code will not be reviewed or merged.
Please also make sure to add/update the usage here: https://github.com/acmesh-official/acme.sh/wiki/dnsapi2
注意: 必须通过了 DNS-API-Test 才会被 review. 无论是修改, 还是新加的 dns api, 都必须确保通过这个测试.

@koelle25
Copy link
Contributor Author

koelle25 commented Aug 7, 2025

DNS-API-Test is not possible! OPNsense is only available in local network, not via GitHub Actions.
But I've done manual tests in my OPNsense GUI/Webinterface, all with success.

@peterkh
Copy link

peterkh commented Aug 9, 2025

Hit the same issue and came across this PR, tested working for me too! thanks @koelle25!

@zv0r
Copy link

zv0r commented Aug 26, 2025

On my local instance I'm using jq to fix this error:

id=$(echo "$_domain_response" | jq -c "[.rows[] | {uuid, enabled, type, domainname}]" | _egrep_o "\"uuid\":\"[a-z0-9\-]*\",\"enabled\":\"1\",\"type\":\"primary\",\"domainname\":\"${h}\"" | cut -d ':' -f 2 | cut -d '"' -f 2)

But I don't think this would be suitable for a pure sh solution, so I didn't make a PR.

@benyamin-codez
Copy link

There's quite a bit more than this to fix.
I included a simpler (imho) fix for this part.
I've finished testing.
I will raise a PR shortly and reference here...

@benyamin-codez
Copy link

I have raised PR #6492, which should cover this and a couple of other fixes... 8^d

@koelle25
Copy link
Contributor Author

koelle25 commented Sep 1, 2025

Closing in favor of #6492

@koelle25 koelle25 closed this Sep 1, 2025
@benyamin-codez
Copy link

Closing in favor of #6492

Thanks Kevin. Hopefully it gets merged before too long...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants